Skip to content

Fix: Reset deployment attempt counter between sessions #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Nov 11, 2025
Merged

Fix: Reset deployment attempt counter between sessions #84

merged 14 commits into from
Nov 11, 2025

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025 •
edited by ineskhou
Loading

Fix Antenna Deployment Test

Description

Issues Fixed here:

Fixed Burnwire end count test

In the integration test, the AntennaDeployer keeps its own tick counter. As soon as its counter reaches the configured duration it calles stopBurn(). If on the third tick the deployer decides it is time to stop and sends the burnStop signal before Burnwire has a chance to run its scheduler callback and increment m_safetyCounter from 2 to 3. BurnwireEndCount(2) instead of 3 so the log of the count will be off if you use BurnwireEnd Count

Solution to count properlly on this is to add the new AntennaBurnSignalCount event to AntennaDeployer to report how many scheduler ticks the burn signal stays active. also Updated the integration test to assert on the new deployer event instead of the burnwire’s BurnwireEndCount.

I reverted the copilot changes.

I removed the copilot additions, changing the this->m_totalAttempts = to init to 0 just makes it count in the same way that m_currentAttempt counts. m_currentAttempt is what is used in the test to count and it gets reset to 0. m_totalAttempts should not be reset to 0 becayse then we can't count how many deploys have happened accross boots which we want as operators

Related Issues/Tickets

Closes #[issue number for antenna deployer test failures]

How Has This Been Tested?

  • Manual testing (logic verification via code simulation)
  • Unit tests (none exist for this component)
  • Integration tests (requires hardware - to be verified by reviewer)
  • Z Tests
  • Manual testing (describe steps)

Test verification needed: Run make test-integration twice without rebooting to confirm tests pass both times.

Checklist

  • Written detailed sdd with requirements, channels, ports, commands, telemetry defined and correctly formatted and spelled
  • Have written relevant integration tests and have documented them in the sdd
  • Have done a code review with
  • Have tested this PR on every supported board with correct board definitions

Further Notes / Considerations

The fix is minimal (1 line) and surgical. Both attempt counters now reset together when starting a new deployment session, eliminating state leakage between test runs.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • astral.sh
    • Triggering command: curl -LsSf REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[BUG] Running Integration Tests More than Once Per Boot Causes Failures in Antenna Deployer</issue_title>
<issue_description>## Summary
If we try to run make test-integration more than once per boot we get the following errors.

================================================ short test summary info ================================================
FAILED FprimeZephyrReference/test/int/antenna_deployer_test.py::test_deploy_without_distance_sensor - AssertionError: First deployment attempt should be attempt Open-Source-Space-Foundation/proves-core-reference#1
FAILED FprimeZephyrReference/test/int/antenna_deployer_test.py::test_burn_duration_sec - AssertionError: First deployment attempt should be attempt Open-Source-Space-Foundation/proves-core-reference#1
============================================= 2 failed, 17 passed in 55.69s =============================================

It seems that the reason for this is that the Antenna Deployer component is saving the attempts between sessions when it shouldn't be.

Steps to Reproduce

  • Flash software onto FC Board
  • Run make test-integration twice without rebooting the board

Environment

  • PROVES Kit V3 Demo Cube</issue_description>

<agent_instructions>You are an expert at solving bugs in F Prime.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix integration tests that fail when run multiple times per boot Fix: Reset deployment attempt counter between sessions Nov 5, 2025
Copilot AI requested a review from Mikefly123 November 5, 2025 20:22
Copilot finished work on behalf of Mikefly123 November 5, 2025 20:22
@ineskhou ineskhou moved this to In review in v0.1-v0.3 Nov 11, 2025
@ineskhou
Copy link
Contributor

ineskhou commented Nov 11, 2025

Issues Fixed here:

  1. Fixed Burnwire end count test

In the integration test, the AntennaDeployer keeps its own tick counter. As soon as its counter reaches the configured duration it calles stopBurn(). If on the third tick the deployer decides it is time to stop and sends the burnStop signal before Burnwire has a chance to run its scheduler callback and increment m_safetyCounter from 2 to 3. BurnwireEndCount(2) instead of 3 so the log of the count will be off if you use BurnwireEnd Count

Solution to count properlly on this is to add the new AntennaBurnSignalCount event to AntennaDeployer to report how many scheduler ticks the burn signal stays active. also Updated the integration test to assert on the new deployer event instead of the burnwire’s BurnwireEndCount.

  1. I reverted the copilot changes.

I removed the copilot additions, changing the this->m_totalAttempts = to init to 0 just makes it count in the same way that m_currentAttempt counts. m_currentAttempt is what is used in the test to count and it gets reset to 0. m_totalAttempts should not be reset to 0 becayse then we can't count how many deploys have happened accross boots which we want as operators

@ineskhou ineskhou marked this pull request as ready for review November 11, 2025 20:55
@ineskhou ineskhou requested a review from hrfarmer November 11, 2025 21:13
hrfarmer
hrfarmer approved these changes Nov 11, 2025
View reviewed changes
Copy link
Collaborator

@hrfarmer hrfarmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ineskhou ineskhou merged commit 22ff9e3 into main Nov 11, 2025
3 checks passed
@ineskhou ineskhou deleted the copilot/fix-integration-test-failures branch November 11, 2025 21:33
@github-project-automation github-project-automation bot moved this from In review to Done in v0.1-v0.3 Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hrfarmer hrfarmer hrfarmer approved these changes

@Mikefly123 Mikefly123 Awaiting requested review from Mikefly123

Assignees

@Mikefly123 Mikefly123

Copilot code review Copilot

Labels

None yet

Projects

v0.1-v0.3
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Running Integration Tests More than Once Per Boot Causes Failures in Antenna Deployer

4 participants

@ineskhou @hrfarmer @Mikefly123